feat: replace Bitnami Keycloak dependency with cloudpirates chart by arnoweiss · Pull Request #46 · eclipse-basyx/charts

arnoweiss · 2026-04-14T08:38:36Z

Summary

Replace bitnami/keycloak@24.4.11 with cloudpirates/keycloak@0.20.0 to resolve Bitnami dependency issues
Slim BaSyx-realm.json and rbac config

Relates: #44

Reviewer Tasks

I've tested these on an internal cluster with slightly different values. deployed and integrated successfully.

helm install deploys without errors
Keycloak starts and imports BaSyx realm successfully
Client credentials flow works for basyx-technical-user and token carries admin realm role
AAS Environment returns 200 with a valid Bearer token

Relates: eclipse-basyx#44 - Replace bitnami/keycloak@24.4.11 with cloudpirates/keycloak@0.20.0 - Slim BaSyx-realm.json to minimal config: admin role, basyx-client-api client roles, basyx-technical-user service account with admin role - Add all RBAC roles from values.yaml to the realm; remove all -two and -three suffixed roles from realm and rbac rules across all services - Set proxyHeaders to xforwarded and ingress className to nginx in values.yaml - Bump chart version to 2.2.0

arnoweiss · 2026-04-14T08:54:58Z

@aaronzi

Copilot

Pull request overview

This PR migrates the umbrella Helm chart’s Keycloak dependency away from Bitnami to the CloudPirates Keycloak chart (addressing the Bitnami artifact/dependency availability concerns in #44), and updates the chart’s default Keycloak configuration and realm/RBAC examples accordingly.

Changes:

Replace the bitnami/keycloak chart dependency with cloudpirates/keycloak (OCI) and bump the umbrella chart version.
Update values.yaml Keycloak settings to match the new chart structure (image, postgres subchart key, ingress structure, realm import settings).
Slim down BaSyx-realm.json and example RBAC blocks; adjust APIRule port and CI repo-add logic for OCI deps.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`charts/basyx/Chart.yaml`	Switch Keycloak dependency to CloudPirates OCI chart and bump chart version.
`charts/basyx/values.yaml`	Update Keycloak values structure (image, postgres key, ingress, service port) and slim RBAC examples.
`charts/basyx/templates/keycloak/basyx-keycloak-apirule.yaml`	Update APIRule to target the Keycloak service port used by the new setup.
`charts/basyx/config/BaSyx-realm.json`	Replace large realm export with a minimal realm/client/role setup for BaSyx.
`.github/workflows/tests.yml`	Prevent `helm repo add` from attempting to add OCI repositories during CI.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-14T09:47:14Z

@@ -106,13 +145,17 @@ keycloak:
    enabled: false
    ## @param keycloak.ingress.ingressClassName IngressClass for the ingress


keycloak.ingress now uses the field className, but the docs still reference keycloak.ingress.ingressClassName. This makes the values file misleading and can break generated documentation. Update the @param annotation (and optionally the label text) to match the actual key (keycloak.ingress.className).

Suggested change

## @param keycloak.ingress.ingressClassName IngressClass for the ingress

## @param keycloak.ingress.className IngressClass for the ingress

Copilot · 2026-04-14T09:47:14Z

  extraVolumes:
    - name: basyx-keycloak-realm
      configMap:
        name: basyx-keycloak-realm


keycloak.fullnameOverride controls the realm ConfigMap name ({{ .Values.keycloak.fullnameOverride }}-realm), but the default extraVolumes[].configMap.name is hard-coded to basyx-keycloak-realm. If a user overrides fullnameOverride, the pod will try to mount a non-existent ConfigMap. Consider making the ConfigMap name stable (not derived from fullnameOverride), or generate the default volume reference in templates so it always matches the rendered ConfigMap name.

made a constant

Copilot · 2026-04-14T09:47:15Z

  service:
    name: {{ .Values.keycloak.fullnameOverride }}
-    port: 80
+    port: 8080


The APIRule service port is hard-coded to 8080, even though values.yaml now defines keycloak.service.httpPort. Hard-coding risks drift if the Keycloak service port is changed via values. Use the configured value (or derive it from the Keycloak subchart service port) instead of a constant.

Suggested change

port: 8080

port: {{ .Values.keycloak.service.httpPort | default 8080 }}

Copilot · 2026-04-14T09:47:15Z

+      "clientId": "basyx-technical-user",
+      "name": "BaSyx Technical User",
+      "description": "Technical service account client for machine-to-machine access with admin role",
+      "enabled": true,
+      "clientAuthenticatorType": "client-secret",
+      "secret": "change-me-pls",
+      "standardFlowEnabled": false,
+      "implicitFlowEnabled": false,
+      "directAccessGrantsEnabled": false,
+      "serviceAccountsEnabled": true,
+      "publicClient": false,


The realm import JSON hard-codes the basyx-technical-user client secret ("change-me-pls"). This makes the default deployment insecure (anyone with chart access can mint admin tokens) and forces users to edit a tracked file to rotate secrets. Please make this secret configurable via values and source it from a Kubernetes Secret (or generate it at install time) rather than committing a fixed credential into the realm file.

added secret parameterized via values.yaml.

aaronzi · 2026-04-14T09:44:06Z

  service:
    name: {{ .Values.keycloak.fullnameOverride }}
-    port: 80
+    port: 8080


Why did this port change?

that's taken care of above - it's configurable now. The defaults exposed by the image changed.

aaronzi · 2026-04-14T09:45:28Z

+- name: keycloak
+  version: "0.20.0"
+  repository: oci://registry-1.docker.io/cloudpirates


Is this the recommended image to use? Is it maintained and are current vulnerabilities considered?

The chart just wraps the image. The image the chart defaults to is the official postgres image [1]. Cloudpirates has been the go-to distribution in the Tractus-X context [2] since bitnami refusing to deliver free updates to their self-hosted images. So we're arguably way better off.

[1] image: docker.io/keycloak/keycloak:26.5.6@sha256:8d44614c7479
[2] https://eclipse-tractusx.github.io/docs/release/trg-5/trg-5-07/

aaronzi · 2026-04-14T09:46:12Z

        run: |
          for dir in $(ls -d charts/*/); do
-            helm dependency list $dir 2> /dev/null | tail +2 | head -n -1 | awk '$3 !~ /^file:/ { print "helm repo add " $1 " " $3 }' | while read cmd; do $cmd; done
+            helm dependency list $dir 2> /dev/null | tail +2 | head -n -1 | awk '$3 !~ /^file:/ && $3 !~ /^oci:/ { print "helm repo add " $1 " " $3 }' | while read cmd; do $cmd; done


What is the impact of this change?

The pipeline not crashing, see https://github.com/eclipse-basyx/charts/actions/runs/24389821173/job/71232645058. OCI-repos don't have to be helm add'ed

aaronzi · 2026-04-14T09:46:57Z

+    httpEnabled: true
+    ## @param keycloak.keycloak.production false = start-dev mode, true = production start
+    ##
+    production: false


Is this intended to be set to false?

aaronzi · 2026-04-14T14:13:38Z

Thanks for the answers and the small fixes @arnoweiss. Looks good to me now. Is this PR ready to merge or do you still want to add/change something?

arnoweiss · 2026-04-14T14:16:47Z

Done from my perspective.

arnoweiss force-pushed the chore/helm-deps branch from 1574c66 to c4651c8 Compare April 14, 2026 08:48

fix: skip helm repo add for OCI-based chart dependencies

3e26446

aaronzi requested a review from Copilot April 14, 2026 09:42

Copilot started reviewing on behalf of aaronzi April 14, 2026 09:42 View session

Copilot AI reviewed Apr 14, 2026

View reviewed changes

aaronzi reviewed Apr 14, 2026

View reviewed changes

fix: address Copilot review comments on keycloak technical user secret

201c345

aaronzi approved these changes Apr 14, 2026

View reviewed changes

aaronzi merged commit 57b9005 into eclipse-basyx:main Apr 14, 2026
2 checks passed

		@@ -106,13 +145,17 @@ keycloak:
		enabled: false
		## @param keycloak.ingress.ingressClassName IngressClass for the ingress

	## @param keycloak.ingress.ingressClassName IngressClass for the ingress
	## @param keycloak.ingress.className IngressClass for the ingress

	port: 8080
	port: {{ .Values.keycloak.service.httpPort \| default 8080 }}

Conversation

arnoweiss commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Reviewer Tasks

Uh oh!

arnoweiss commented Apr 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

arnoweiss Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aaronzi commented Apr 14, 2026

Uh oh!

arnoweiss commented Apr 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

arnoweiss commented Apr 14, 2026 •

edited

Loading

arnoweiss Apr 14, 2026 •

edited

Loading